feat(sandbox): enforce proxy-aware network routing in sandbox#11113
feat(sandbox): enforce proxy-aware network routing in sandbox#11113viyatb-oai wants to merge 9 commits intomainfrom
Conversation
viyatb-oai
force-pushed
the
codex/viyatb/sandbox-proxy-wiring
branch
from
e3a69d3 to
f16c393
Compare
bolinfest
force-pushed
the
pr11105
branch
3 times, most recently
from
a26207b to
954e0d1
Compare
viyatb-oai
changed the title
bolinfest
force-pushed
the
pr11105
branch
5 times, most recently
from
1c3bade to
714e595
Compare
viyatb-oai
force-pushed
the
codex/viyatb/sandbox-proxy-wiring
branch
2 times, most recently
from
d2b8198 to
c5b31de
Compare
viyatb-oai
force-pushed
the
codex/viyatb/sandbox-proxy-wiring
branch
from
c5b31de to
20006b9
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8862494880
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let install_network_seccomp = | ||
| !sandbox_policy.has_full_network_access() && !allow_network_for_proxy; | ||
|
|
||
| // `PR_SET_NO_NEW_PRIVS` is required for seccomp, but it also prevents | ||
| // setuid privilege elevation. Many `bwrap` deployments rely on setuid, so | ||
| // we avoid this unless we need seccomp or we are explicitly using the | ||
| // legacy Landlock filesystem pipeline. | ||
| if !sandbox_policy.has_full_network_access() | ||
| if install_network_seccomp | ||
| || (apply_landlock_fs && !sandbox_policy.has_full_disk_write_access()) | ||
| { | ||
| set_no_new_privs()?; | ||
| } | ||
|
|
||
| if !sandbox_policy.has_full_network_access() { | ||
| if install_network_seccomp { | ||
| install_network_seccomp_filter_on_current_thread()?; |
There was a problem hiding this comment.
On Linux, allow_network_for_proxy disables the network seccomp filter for any non-empty proxy env var, which means a restricted SandboxPolicy now allows unrestricted network syscalls whenever HTTP_PROXY/etc. is present. A sandboxed process can then ignore the proxy env and open direct connections, so this change removes the previous network isolation guarantee (regression from “no network” to “full network”) even if the proxy URL is invalid or points off-host. Consider keeping network seccomp in place (or otherwise enforcing proxy-only egress) instead of treating any proxy env as a safe bypass.
Useful? React with 👍 / 👎.
nornagon-openai
left a comment
nornagon-openai
left a comment
There was a problem hiding this comment.
sandbox changes seem safe in that they are strictly less permissive than previously. didn't review the rest of the PR in depth.
2 participants
Summary
HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXYfamilies + tool-specific variants)